-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ZFPY import error #540
Conversation
When numpy >=2.0.0 from numcodecs.zfpy import ZFPY raise error [ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject] The error is caused by zfpy library crashing with recent ABI breaking of numpy udpate. So we'd better force numpy <2.0 unless zfpy update again. Reference: (https://stackoverflow.com/questions/66060487/valueerror-numpy-ndarray-size-changed-may-indicate-binary-incompatibility-exp)
We want to support numpy 2.0. Unfortunately, that means we need to wait until zfpy has been updated. See #535 |
Agreed That said, do understand why this error is confusing Perhaps we can add some logic here to check NumPy version and disable the extension with an appropriate message to the user Lines 3 to 5 in 342754d
We could also ensure that the Lines 65 to 67 in 342754d
|
Add warning when user import zfpy but with numpy >=2.0.0
ensure that the zfpy optional dependency has this constraint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 🙏
Am wondering if we should check whether zfpy
exists before warning
Also wondering if we should limit which zfpy
versions warn (since at some point this will be fixed)
What do you think? 🙂
Co-authored-by: jakirkham <[email protected]>
Co-authored-by: jakirkham <[email protected]>
Thanks! 🙏 Please add a release note as well. Perhaps under fix? Lines 27 to 33 in 342754d
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #540 +/- ##
==========================================
- Coverage 99.91% 99.82% -0.09%
==========================================
Files 59 59
Lines 2319 2329 +10
==========================================
+ Hits 2317 2325 +8
- Misses 2 4 +2
|
pre-commit.ci autofix |
This looks like a great solution. Thank you! |
@jakirkham Thank you for constructive suggestions, and thanks @normanrz for mention #535 It helps the numpy 2.0 users like me who would like to use zfpy but no clues where is going wrong with import error |
It looks like this PR was merged without hitting the coverage target. Now all subsequent PRs to numcodecs are failing that test (see #544 (comment)). We should fix this. |
Fixed in #546 |
Thanks all! 🙏 |
* Update pyproject.toml When numpy >=2.0.0 from numcodecs.zfpy import ZFPY raise error [ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject] The error is caused by zfpy library crashing with recent ABI breaking of numpy udpate. So we'd better force numpy <2.0 unless zfpy update again. Reference: (https://stackoverflow.com/questions/66060487/valueerror-numpy-ndarray-size-changed-may-indicate-binary-incompatibility-exp) * Update zfpy.py Add warning when user import zfpy but with numpy >=2.0.0 * Update pyproject.toml ensure that the zfpy optional dependency has this constraint. * Update zfpy.py * Update numcodecs/zfpy.py Co-authored-by: jakirkham <[email protected]> * Update numcodecs/zfpy.py Co-authored-by: jakirkham <[email protected]> * Update zfpy.py * Update release.rst * Update release.rst * style: pre-commit fixes * Update zfpy.py --------- Co-authored-by: jakirkham <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
When
numpy >=2.0.0
from numcodecs.zfpy import ZFPY
raise error [ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject]
The error is caused by zfpy library crashing with recent ABI breaking of numpy udpate. So we'd better force numpy <2.0 unless zfpy update again.
Reference: (https://stackoverflow.com/questions/66060487/valueerror-numpy-ndarray-size-changed-may-indicate-binary-incompatibility-exp)
[Description of PR]
TODO: